update to literal-escaper 0.0.4 for better API without `unreachable` and faster string parsing #140999

hkBst · 2025-05-14T10:24:03Z

This is the replacement for just the part of #138163 dealing with the changed API of unescape functionality, since that got moved into its own crate.

~~This uses an unpublished version of literal-escaper (rust-lang/literal-escaper#8).~~

r? @nnethercote

bors · 2025-05-16T13:06:45Z

☔ The latest upstream changes (presumably #141044) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2025-05-29T16:09:52Z

☔ The latest upstream changes (presumably #141595) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot · 2025-06-13T12:35:51Z

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

These commits modify the library/Cargo.lock file. Unintentional changes to library/Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

GuillaumeGomez · 2025-06-13T12:37:48Z

Let's run a perf check then.

@bors try @rust-timer queue

bors · 2025-06-13T12:38:58Z

⌛ Trying commit c15bc48 with merge 74c9a54...

update to literal-escaper 0.0.4 for better API without `unreachable` and faster string parsing This is the replacement for just the part of #138163 dealing with the changed API of unescape functionality, since that got moved into its own crate. This is a draft, because it uses an unpublished version of literal-escaper (rust-lang/literal-escaper#8). To test, clone literal-escaper into a folder next to rustc, and test rustc normally. r? `@nnethercote`

lnicola · 2025-06-13T12:51:22Z

Please file the rust-analyzer parts upstream once it's released, if possible.

bors · 2025-06-13T13:04:18Z

💔 Test failed - checks-actions

rust-bors · 2025-06-13T17:46:50Z

☀️ Try build successful (CI)
Build commit: 8021674 (80216743568d271c50d695bcafdbdfc896378b2b, parent: c35911781925bcbfdeb5e6e1adb305097af46801)

hkBst · 2025-06-13T18:46:08Z

This works locally, but I'm confused about the CI failure, but try build success...

GuillaumeGomez · 2025-06-13T18:50:02Z

Well, as long as the benches have been started, all good. :)

rust-timer · 2025-06-13T20:36:37Z

Finished benchmarking commit (8021674): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	0.2%	[0.1%, 0.3%]	4
Regressions ❌ (secondary)	0.1%	[0.1%, 0.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.6%	[-1.2%, -0.1%]	17
All ❌✅ (primary)	0.2%	[0.1%, 0.3%]	4

Max RSS (memory usage)

Results (primary -6.2%, secondary 2.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.6%	[2.6%, 2.6%]	1
Improvements ✅ (primary)	-6.2%	[-6.2%, -6.2%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-6.2%	[-6.2%, -6.2%]	1

Cycles

Results (secondary 0.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.8%	[2.8%, 2.8%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.1%	[-2.1%, -2.1%]	1
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 755.253s -> 755.043s (-0.03%)
Artifact size: 372.23 MiB -> 372.28 MiB (0.01%)

GuillaumeGomez · 2025-06-16T11:32:29Z

Cargo.toml

@@ -89,3 +89,6 @@ codegen-units = 1
 # FIXME: LTO cannot be enabled for binaries in a workspace
 # <https://github.com/rust-lang/cargo/issues/9330>
 # lto = true
+
+[patch.crates-io]


Can be removed.

True, but it may be nice to have anyway, like in src/tools/rust-analyzer/Cargo.toml

Why? If it's not in use it should not be there. You can leave a comment explaining what to use in case a similar situation happen in the future though.

Yeah, should be dropped, but I'd prefer these parts to be filed as a PR in the rust-analyzer repo. I don't think it will cause issues except maybe a small, temporary, build slowdown (since it's compiling two versions).

Would it make a difference if I comment out "[patch.crates-io]"?

I guess it's fine but again: please add an explanation why this is kept around.

I added an explanatory comment. Let me know if this is okay, or if I should remove the lot.

hkBst · 2025-06-16T12:00:48Z

Please file the rust-analyzer parts upstream once it's released, if possible.

@lnicola While this is technically possible, I'm not sure how to transplant these changes efficiently. Is there maybe documentation for doing that? Or would it be possible for you to review here?

lnicola · 2025-06-16T12:10:33Z

I'm not sure how to transplant these changes efficiently. Is there maybe documentation for doing that?

I downloaded https://github.com/rust-lang/rust/pull/140999.patch, deleted most of the changes, including the rust-analyzer Cargo.lock and Cargo.toml, applied it with git am and updated Cargo.toml manually since we bumped it to 0.0.3 in the meanwhile. I had to update the file paths (to remove src/tools/rust-analyzer/), but there was no need to change the file list at the top.

GuillaumeGomez · 2025-06-16T14:16:23Z

Cargo.toml

+# If you want to use a crate with local modifications, you can set a path or git dependency here.
+# For git dependencies, also add your source to ALLOWED_SOURCES in src/tools/tidy/src/extdeps.rs.
+[patch.crates-io]
+# rustc-literal-escaper = { path = '../literal-escaper/' }


This one can be removed now, right?

lnicola · 2025-06-16T14:29:14Z

Given rust-lang/rust-analyzer#20012, can we drop the rust-analyzer parts?

…and faster string parsing

hkBst · 2025-06-16T17:35:38Z

Given rust-lang/rust-analyzer#20012, can we drop the rust-analyzer parts?

Now without rust-analyzer parts.

rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 14, 2025

hkBst mentioned this pull request May 14, 2025

Remove unreachable rust-lang/literal-escaper#8

Merged

rust-cloud-vms bot force-pushed the update-escaper branch from 7db948b to e61a48c Compare May 14, 2025 11:13

rustbot assigned nnethercote May 14, 2025